Skip to content

Prevent infinite recursion and OOM in project_and_unify_term#156842

Open
BloodStainedCrow wants to merge 1 commit into
rust-lang:mainfrom
BloodStainedCrow:issue-156615-fix
Open

Prevent infinite recursion and OOM in project_and_unify_term#156842
BloodStainedCrow wants to merge 1 commit into
rust-lang:mainfrom
BloodStainedCrow:issue-156615-fix

Conversation

@BloodStainedCrow

@BloodStainedCrow BloodStainedCrow commented May 22, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown

Prevents an infinite recursion when using the old trait solver.

Fixes #156615.

Note that the example in #156615 encounters this infinite recursion in report_ambiguity_errors.
The overflow abort introduced here shadows the original error for which we are generating a report, which is unfortunate but preferable to hangs/OOMs:

# Notably no info about "type ambiguity"
error[E0275]: overflow evaluating the requirement `<Family as DistributionFamily>::Distribution<_> == _`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0275`.

Ideally, we could report the original error AND report the overflow while generating the report, something like this:

error[E0283]: type annotations needed
  --> non-termination-while-reporting-ambiguity-error-issue-1156615.rs:28:10
  --> error: overflow evaluating the requirement `<Family as DistributionFamily>::Distribution<_> == _` while generating error report

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 22, 2026
@rustbot

rustbot commented May 22, 2026

Copy link
Copy Markdown
Collaborator

r? @dingxiangfei2009

rustbot has assigned @dingxiangfei2009.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 19 candidates

@BloodStainedCrow BloodStainedCrow changed the title Prevent infinite recursion and OOM in project_and_unify_term Prevent infinite recursion and OOM in project_and_unify_term May 22, 2026
@BloodStainedCrow

BloodStainedCrow commented May 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

I have added the recursion check to the "lowest" function which added the new obligation,
according to this comment "process_projection_obligation should do its own recursion depth checking" but from what I can tell, nothing in there does any checking? Is that intentional?

So it might make sense to move this check further up to cover more of process_projection_obligation but I do not have enough understanding of the type checking being done here to know which functions could theoretically recurse infinitely.

@BloodStainedCrow

BloodStainedCrow commented Jun 5, 2026

Copy link
Copy Markdown
Author

@rustbot reroll

@rustbot rustbot assigned mati865 and unassigned dingxiangfei2009 Jun 5, 2026
@mati865

mati865 commented Jun 10, 2026

Copy link
Copy Markdown
Member

Sorry for the long wait. I'm not familiar with the code here enough to suggest anything here, but I might be able to find somebody more knowledgeable.

r? project-trait-system-refactor

BTW amazing work on your FactoryGame project ;)

@rustbot rustbot assigned cjgillot and unassigned mati865 Jun 10, 2026
@mati865

mati865 commented Jun 10, 2026

Copy link
Copy Markdown
Member

cjgillot you have 20 PRs assigned, so let me know if I should look for somebody else

@rust-bors

rust-bors Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #157894) made this pull request unmergeable. Please resolve the merge conflicts.

@cjgillot

cjgillot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

I'm not confident to review this
r? types

@rustbot rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Jun 24, 2026
@rustbot rustbot assigned lcnr and unassigned cjgillot Jun 24, 2026
@lcnr

lcnr commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

I have added the recursion check to the "lowest" function which added the new obligation,
according to this comment "process_projection_obligation should do its own recursion depth checking" but from what I can tell, nothing in there does any checking? Is that intentional?

we should handle this limit in project itself

rust/compiler/rustc_trait_selection/src/traits/project.rs

Lines 697 to 703 in 63f05e3

if !selcx.tcx().recursion_limit().value_within_limit(obligation.recursion_depth) {
// This should really be an immediate error, but some existing code
// relies on being able to recover from this.
return Err(ProjectionError::TraitSelectionError(SelectionError::Overflow(
OverflowError::Canonical,
)));
}

Where does the cycle occur to never trigger this?

Does the overflow happen due to nested obligations from equating the output of project with the expected term?

lcnr
lcnr reviewed Jun 29, 2026
View reviewed changes
Comment thread compiler/rustc_trait_selection/src/traits/project.rs
if !selcx.tcx().recursion_limit().value_within_limit(obligation.recursion_depth) {
selcx.infcx.err_ctxt().report_overflow_obligation(&obligation, false);
}
}

@lcnr lcnr Jun 29, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like the overflow happens because inferred_obligations contains another Projection obligation which ends up causing the same thing yet again?

I would maybe just copy the overflow check of project to also happen at the start of this function?

View changes since the review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lcnr lcnr lcnr left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@lcnr lcnr

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Switching to Family-Pattern results in compiler non-termination in report_ambiguity_errors

6 participants

@BloodStainedCrow @rustbot @mati865 @cjgillot @lcnr @dingxiangfei2009